Allow drag-and-dropping multiple containers and views in the blueprint tree #8334
Conversation
abey79
added
enhancement
|
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
abey79
force-pushed
the
antoine/dnd-multi-select-content
branch
from
a07e4e5 to
219b298
Compare
|
I haven't been able to actually drop any dragged entities yet. Is that intended to work here or is it just dragging? |
Yeah this PR is just about views/container but lays the groundwork. Support for dragging entities will come next. |
|
Ignore for now if this is just super premature design feedback but I don't like the experience of showing an error all the time. If we want to show an error on the drag-pill it feels like that should be reserved for situations where I clearly want to drop it somewhere but that doesn't work. drag-and-drop.mp4 |
|
I expected the result of both these kinds of drags to have an effect: no-ordering-effect.mp4no-effect-2.mp4 |
Yeah it broke it apparently. I'll fix it before this gets undrafted. |
- add `undraggable_items` to `ViewerContext` and populate with root container
abey79
force-pushed
the
antoine/dnd-multi-select-content
branch
from
002b15f to
1c09ca9
Compare
# Conflicts: # crates/viewer/re_view_bar_chart/src/view_class.rs # crates/viewer/re_view_graph/src/ui/draw.rs # crates/viewer/re_view_map/src/map_view.rs # crates/viewer/re_viewer/src/app_state.rs # crates/viewer/re_viewer_context/src/contents.rs # examples/rust/custom_view/src/color_coordinates_view.rs
| &response, | ||
| Item::DataResult(query.view_id, instance_path.clone()), | ||
| false, |
There was a problem hiding this comment.
Instead of adding false and true as argument, did you consider having two different functions, for increased readability? In particular, it is very hard for me reviewing this code to know what false means.
I.e. you could have kept the old select_hovered_on_click with the previous meaning, and added handle_select_hover_drag_interactions (without a bool) for draggable things. But maybe its better to force each users to consider wether or not an item is draggable in this context?
There was a problem hiding this comment.
Yes, I want people to think of draggability and make a call when using this function.
It's also a toggle that allows me to progressively implement draggability one UI area at a time (e.g. next PR will switch some items to true in the streams tree).
There is an ideal version of the future where this is true everywhere (and thus the arg can be removed), but I'm not sure this is realistic. There are cases where stuff should hover/select highlight, but not be draggable. For example, entity buttons should probably not be draggable—or at least not always (e.g. from the add/remove entity modal).
In generally, we should make something draggable only if it makes sense, e.g if there is an obvious drop destination.
I'll improve the docstring to expand on the above. Let me know if you have any further suggestion on this.
|
Sometimes when you drag things you get a pill, sometimes you don't
|
That's a design decision we made because the red pill was worse. The lack of pill is feedback that the current selection is not draggable. As for this specific view: the next PR will enable dragging entities, so with it, every item in the blueprint tree will be individually draggable (or in homogenous multiple selection). This is not generally true however. For example, in the streams tree the entities will be draggable, but not the components. |
Co-authored-by: Emil Ernerfeldt <[email protected]>
| /// If a selection contains any `undraggable_items`, it may not be dragged. | ||
| pub undraggable_items: &'a ItemCollection, |
There was a problem hiding this comment.
You can at least document it here exactly what it is was introduced for, and maybe with a TODO for "this is ugly; if you have a better idea please rewrite this" :]
Related
What
This PR makes it possible to drag multi-selection of views and containers within the blueprint tree. It lays the foundation of a system that will be extended to other drag payload and UI sections.
Specifically:
ctx.handle_select_hover_drag_interactions()(formerlyctx.select_hovered_on_click()) is now able to initiate drag interactions. This is opt-in for now, as dragging from most place isn't supported yet (to implemented in follow-up PRs).DragAndDropPayloadtype to interoperate between various part of the UI. This type also enforce the grouping of items that can meaningfully be dragged together (e.g. it's ok to drag a view and a container together, because there exist somewhere they can be dropped to, but it's not ok to drag a view and an entity together).master.